Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Distribute docker container to run ATH #238

Merged
merged 17 commits into from
Aug 16, 2017

Conversation

olivergondza
Copy link
Member

Add Dockerfile to build container ATH can run in - no only on ci.jenkins.io.

@rtyler, I put the form-element-path-plugin on hold as it will reuse large part of this. Please review and enable on ci.jenkins.io once you are ok with this.

@mafraba, review appreciated. Btw, this is for the first time I got ATH running in container and it seems to work fine without SHARED_DOCKER_SERVICE=true, at least for xvnc plugin test. Do I miss something?

docs/DOCKER.md Outdated
... // Checkout ATH
sh 'docker build --build-arg=uid=$(id -u) --build-arg=gid=$(id -g) -t jenkins/ath src/main/resources/ath-container'
docker.image('jenkins/ath').inside {
sh 'eval $(./vnc.sh > /dev/null) && ./run.sh firefox latest -Dmaven.test.failure.ignore=true -DforkCount=1 -B'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The redirection to /dev/null is not part of your change to Jenkinsfile above.

Jenkinsfile Outdated
changelog scm

sh 'docker build --build-arg=uid=$(id -u) --build-arg=gid=$(id -g) -t jenkins/ath src/main/resources/ath-container'
docker.image('jenkins/ath').inside {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to add this:

String containerArgs = '-v /var/run/docker.sock:/var/run/docker.sock -v $HOME/.m2:/var/maven/.m2'

and change it to be:

docker.image('jenkins/ath').inside(containerArgs) {

Otherwise, docker ps will not be able to connect to the docker daemon and docker-based test will fail

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The maven repo is shared for optimization only, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Forgot to mention that.

Jenkinsfile Outdated
sh 'docker build --build-arg=uid=$(id -u) --build-arg=gid=$(id -g) -t jenkins/ath src/main/resources/ath-container'
docker.image('jenkins/ath').inside {
sh '''
eval $(./vnc.sh > /dev/null 2>&1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be:

eval $(./vnc.sh 2>/dev/null)

otherwise the echo export BROWSER_DISPLAY=$display has no effect

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vnc.sh, needs its output handling reworked I guess.

@scoheb
Copy link
Contributor

scoheb commented Dec 9, 2016

I would also maybe adjust vnc.sh to have this code:

if [ -x "$(which vncviewer)" ] ; then
    (vncviewer $display || vncviewer localhost$display) > /dev/null &
fi

no need to execute vncviewer blindly if not installed.

@mafraba
Copy link
Contributor

mafraba commented Dec 14, 2016

@olivergondza wrt SHARED_DOCKER_SERVICE property, I guess it can work without it as long as the docker port command returns the correct IP and the docker host and mapped ports are accessible. But it shouldn't be necessary to access the containers through mapped ports in the host anymore, since they can be accessed directly thus avoiding potential connectivity issues.

Jenkinsfile Outdated
]) {
changelog scm

sh 'docker build --build-arg=uid=$(id -u) --build-arg=gid=$(id -g) -t jenkins/ath src/main/resources/ath-container'
Copy link
Contributor

@scoheb scoheb Jan 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As @rtyler suggested, here is the code to make use of docker.build():

def uid = sh returnStdout: true, script: "id -u | tr '\n' ' '"
def gid = sh returnStdout: true, script: "id -g | tr '\n' ' '"

def buildArgs = "--build-arg=uid=${uid} --build-arg=gid=${gid} src/main/resources/ath-container"
docker.build('jenkins/ath', buildArgs)

@olivergondza
Copy link
Member Author

@rtyler can you create the project on ci.jenkins.io so we get this thing moving?

@rtyler
Copy link
Member

rtyler commented Feb 15, 2017

If there isn't an INFRA ticket already, please file one.

@olivergondza
Copy link
Member Author

Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have not personally tried it but sounds like a good idea.

@@ -9,6 +9,8 @@ fi

vncserver -kill $display
vncserver -geometry 1750x1250 $display > /dev/null
vncviewer localhost$display > /dev/null &
if command -v vncviewer >/dev/null 2>&1; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should expose port 9042 or whatever it is, so people can attach a desktop vncviewer to the container and watch the magic happen.

Jenkinsfile Outdated
node('highram&&docker') {
withEnv([
"JAVA_HOME=${tool jdkName}",
"PATH+MAVEN=${tool mavenName}/bin:${env.JAVA_HOME}/bin"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need these at all, if the build is done inside the image anyway?

Jenkinsfile Outdated
def gid = sh returnStdout: true, script: "id -g | tr -d '\n'"

def buildArgs = "--build-arg=uid=${uid} --build-arg=gid=${gid} src/main/resources/ath-container"
docker.build('jenkins/ath', buildArgs)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assign the return value to a local variable and use in place of docker.image('jenkins/ath') below.

openjdk-8-jdk \
vnc4server

# Allow injecting uid and git to match directory ownership
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is sufficiently weird you had better document clearly why this is necessary.

Jenkinsfile Outdated
def buildArgs = "--build-arg=uid=${uid} --build-arg=gid=${gid} src/main/resources/ath-container"
docker.build('jenkins/ath', buildArgs)

String containerArgs = '-v /var/run/docker.sock:/var/run/docker.sock'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might need --group-add=${gid} to make sure docker.sock is writable. Unclear to me whether that is handled already by the other tricks.

docs/DOCKER.md Outdated
$ eval $(./vnc.sh)
$ ./run.sh firefox latest -Dmaven.test.failure.ignore=true -DforkCount=1 -B

Jenkinsfile:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems redundant. Anyone who cares can look up the Jenkinsfile for themselves.

RUN apt-get -y update && \
apt-get install -y \
curl \
docker.io \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also just download a specific client executable.

RUN touch /home/ath-user/.vnc/xstartup && chmod a+x /home/ath-user/.vnc/xstartup

# Set SUID and SGID for docker binary so it can communicate with mapped socket its uid:gid we can not control. Alternative
# approach used for this is adding ath-user to the group of /var/run/docker.sock but that require root permission we do not
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hence --group-add. Maybe.

@jglick jglick changed the title Distribute docker contianer to run ATH Distribute docker container to run ATH Mar 31, 2017
@scoheb
Copy link
Contributor

scoheb commented Apr 20, 2017

Stole your Dockerfile and part of Jenkinsfile for use in jms-messaging-plugin. Working well now. Thanks!

@olivergondza
Copy link
Member Author

Cool. Be prepared this will receive several updates before it is ready.

@olivergondza
Copy link
Member Author

Let's get this merged to so see it run and fail so we can fix as we go.

@olivergondza olivergondza merged commit 5baca37 into jenkinsci:master Aug 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants